Skip to content

ci: add workflow_dispatch#4108

Draft
shubhdoshi21 wants to merge 2 commits intoresilient-tech:developfrom
shubhdoshi21:ci-workflow_dispatch
Draft

ci: add workflow_dispatch#4108
shubhdoshi21 wants to merge 2 commits intoresilient-tech:developfrom
shubhdoshi21:ci-workflow_dispatch

Conversation

@shubhdoshi21
Copy link
Copy Markdown
Member

@shubhdoshi21 shubhdoshi21 commented Mar 18, 2026

Add workflow_dispatch with Custom Frappe/ERPNext Remotes and Concurrency control

Adds a manual workflow_dispatch trigger to server-tests.yml so the CI can be run against custom forks or branches of Frappe and ERPNext without modifying any files. Also fixes install.sh to correctly handle ERPNext forks that have a different app/repo name than erpnext.

Adds concurrency group keyed by event type + PR number (or run ID for dispatches), with cancel-in-progress: true to avoid redundant runs on the same PR.

@karm1000
Copy link
Copy Markdown
Member

karm1000 commented Apr 6, 2026

@shubhdoshi21 resolve conflicts

@shubhdoshi21 shubhdoshi21 force-pushed the ci-workflow_dispatch branch from 247debc to dc21e74 Compare April 6, 2026 16:49
@codacy-production
Copy link
Copy Markdown

codacy-production bot commented Apr 6, 2026 •
edited
Loading

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

TIP This summary will be updated as you push new changes. Give us feedback

@shubhdoshi21 shubhdoshi21 force-pushed the ci-workflow_dispatch branch 2 times, most recently from a89a7dc to e27dff2 Compare April 6, 2026 16:59
@shubhdoshi21 shubhdoshi21 force-pushed the ci-workflow_dispatch branch from e27dff2 to 209231e Compare April 6, 2026 17:08
@ljain112 ljain112 marked this pull request as ready for review April 7, 2026 13:19
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 7, 2026

Confidence Score: 4/5

Not safe to merge as-is — the concurrency misconfiguration will cancel cross-branch push runs.

One P1 logic bug: all push events resolve to the same concurrency group (server-tests-push-), so concurrent pushes to any of the 7 protected branches will cancel each other. The install.sh changes are correct and the workflow_dispatch wiring is clean.

.github/workflows/server-tests.yml — specifically the concurrency group expression at line 69.

Important Files Changed
Filename Overview
.github/workflows/server-tests.yml Adds workflow_dispatch trigger and concurrency control; concurrency group collapses all push-event runs into the same group, causing cross-branch cancellations
.github/helper/install.sh Replaces hardcoded frappe/erpnext URLs with env-var overrides using correct :- bash defaults; logic is sound

Reviews (1): Last reviewed commit: "ci: add workflow_dispatch" | Re-trigger Greptile

greptile-apps[bot]
greptile-apps bot reviewed Apr 7, 2026
View reviewed changes
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 7, 2026

📝 Walkthrough

Walkthrough

The pull request introduces parameterization of Git remotes and branches for frappe and erpnext across the installation and workflow infrastructure. The install script (.github/helper/install.sh) now accepts environment variables (FRAPPE_REMOTE, FRAPPE_BRANCH, ERPNEXT_REMOTE, ERPNEXT_BRANCH) to configure the sources for cloning and fetching these repositories, with defaults preserving the original behavior. The server tests workflow (.github/workflows/server-tests.yml) adds a workflow_dispatch trigger exposing these configuration options as manual input parameters and wires them into the install step. Additionally, a concurrency configuration was introduced to cancel overlapping workflow runs.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'ci: add workflow_dispatch' is vague and generic; it does not clearly convey what the workflow_dispatch addition accomplishes or its main benefit (custom remote/branch support and concurrency control). Consider a more descriptive title that highlights the key functionality, such as 'ci: add workflow_dispatch for custom Frappe/ERPNext remotes and concurrency control'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the purpose of the changes: adding workflow_dispatch for custom forks, fixing install.sh for ERPNext app name handling, and adding concurrency control—all of which are reflected in the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot reviewed Apr 7, 2026
View reviewed changes
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5a4e86b7-e93b-4161-ba89-8d4c22d854a2

📥 Commits

Reviewing files that changed from the base of the PR and between f6d7c6f and 209231e.

📒 Files selected for processing (2)
  • .github/helper/install.sh
  • .github/workflows/server-tests.yml

@ljain112 ljain112 marked this pull request as draft April 7, 2026 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

@priyanshshah2442 priyanshshah2442 Awaiting requested review from priyanshshah2442

@sagarvora sagarvora Awaiting requested review from sagarvora

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@shubhdoshi21 @karm1000